Skip to content

fix(network): Verify accepted type of incoming game messages#2708

Merged
xezon merged 5 commits into
TheSuperHackers:mainfrom
stephanmeesters:fix/gamemessage-network-type
May 20, 2026
Merged

fix(network): Verify accepted type of incoming game messages#2708
xezon merged 5 commits into
TheSuperHackers:mainfrom
stephanmeesters:fix/gamemessage-network-type

Conversation

@stephanmeesters
Copy link
Copy Markdown

Currently outgoing GameMessage's are checked using Network::isTransferCommand but incoming messages are not checked, which can potentially enable certain cheats. Credits to Caball for pointing this out.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 14, 2026

Greptile Summary

This PR adds incoming GameMessage type validation in Network::RelayCommandsToCommandList to mirror the existing outgoing-direction check in GetCommandsFromCommandList, closing a potential exploit path where a remote peer could inject non-network message types. isTransferCommand is refactored into a cleaner static helper isMessageTypeWithinNetworkRange that operates on the enum value directly rather than a pointer, and a new getGameMessageType() accessor is added to NetGameCommandMsg to enable the pre-construction type check.

  • Ingress validation (Network.cpp): The new check reads m_type via getGameMessageType() before calling constructGameMessage(), which avoids an unnecessary allocation for rejected messages and validates the raw deserialized enum value at the right boundary.
  • Static helper refactor: isMessageTypeWithinNetworkRange drops the null-pointer branch (correct, since callers are now responsible for passing a valid Type value) and is marked static, removing the implicit this dependency.
  • Compile-time gate: The incoming validation is wrapped in #if !RETAIL_COMPATIBLE_CRC, which defaults to 1 (disabled) — the anti-cheat check is inactive in standard community builds; this was already flagged in a prior review thread.

Confidence Score: 4/5

The change is safe to merge — it cannot regress existing behaviour because the new validation path is only active when RETAIL_COMPATIBLE_CRC is explicitly set to 0, which is a non-default configuration.

The fix's core goal — blocking incoming messages with out-of-range types — is guarded by #if RETAIL_COMPATIBLE_CRC and therefore inactive in every default build. The refactor to isMessageTypeWithinNetworkRange and the new getGameMessageType() accessor are both correct and safe; the concern is solely that the anti-cheat protection this PR was created to deliver does not fire in the builds most users will run.

Core/GameEngine/Source/GameNetwork/Network.cpp — the validation logic inside the #else branch deserves a second look once the project decides to lift the retail compatibility constraint.

Important Files Changed

Filename Overview
Core/GameEngine/Include/GameNetwork/NetCommandMsg.h Adds getGameMessageType() accessor declaration to NetGameCommandMsg; straightforward getter addition.
Core/GameEngine/Source/GameNetwork/NetCommandMsg.cpp Implements getGameMessageType(), returning m_type directly — correct and consistent with the companion setter.
Core/GameEngine/Source/GameNetwork/Network.cpp Renames isTransferCommand to the static isMessageTypeWithinNetworkRange, and adds incoming-message type validation gated behind #if !RETAIL_COMPATIBLE_CRC; validation is dead code in default builds where RETAIL_COMPATIBLE_CRC=1.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Receive NetCommandRef from ConnectionManager] --> B{cmdType == NETCOMMANDTYPE_GAMECOMMAND?}
    B -- No --> C[processFrameSynchronizedNetCommand]
    B -- Yes --> D[Cast to NetGameCommandMsg]
    D --> E{RETAIL_COMPATIBLE_CRC == 1?}
    E -- Yes default --> F[appendMessage unconditionally]
    E -- No --> G{isMessageTypeWithinNetworkRange\ngetGameMessageType}
    G -- In range --> H[constructGameMessage\nappendMessage to TheCommandList]
    G -- Out of range --> I[DEBUG_LOG rejection\nDrop message]
Loading

Reviews (7): Last reviewed commit: "Rework to check message type before crea..." | Re-trigger Greptile

Comment thread Core/GameEngine/Source/GameNetwork/Network.cpp Outdated
Comment thread Core/GameEngine/Source/GameNetwork/Network.cpp Outdated
Comment thread Core/GameEngine/Source/GameNetwork/Network.cpp Outdated
Comment thread Core/GameEngine/Source/GameNetwork/Network.cpp
@Caball009 Caball009 added Minor Severity: Minor < Major < Critical < Blocker Network Anything related to network, servers labels May 14, 2026
Comment thread Core/GameEngine/Source/GameNetwork/Network.cpp Outdated
@xezon xezon changed the title fix(gamemessage): Verify allowed network type of incoming GameMessages fix(network): Verify allowed network type of incoming GameMessages May 15, 2026
@xezon xezon changed the title fix(network): Verify allowed network type of incoming GameMessages fix(network): Verify accepted type of incoming game messages May 15, 2026
@xezon xezon added Fix Is fixing something, but is not user facing NoRetail This fix or change is not applicable with Retail game compatibility labels May 15, 2026
Comment thread Core/GameEngine/Source/GameNetwork/Network.cpp Outdated
Copy link
Copy Markdown

@xezon xezon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good.

@xezon
Copy link
Copy Markdown

xezon commented May 20, 2026

Rebase this just to make sure.

@stephanmeesters stephanmeesters force-pushed the fix/gamemessage-network-type branch from 8c4722a to 458c556 Compare May 20, 2026 10:37
@stephanmeesters
Copy link
Copy Markdown
Author

Rebase this just to make sure.

Done

@xezon xezon merged commit 5a4187c into TheSuperHackers:main May 20, 2026
17 checks passed
@stephanmeesters stephanmeesters deleted the fix/gamemessage-network-type branch May 20, 2026 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Fix Is fixing something, but is not user facing Minor Severity: Minor < Major < Critical < Blocker Network Anything related to network, servers NoRetail This fix or change is not applicable with Retail game compatibility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants